Return false on timeout in waitForExit

waitForExit always resolves true because Promise.race([once(child,"exit"), delay(timeoutMs)]) never rejects; the delay path resolves after the timeout. When the child ignores SIGTERM, stop() will think it exited and skip the SIGKILL path, leaving the process running and potentially holding ports for later tests. Make the timeout path resolve false (or reject) so the SIGKILL fallback actually runs when the process doesn’t exit in time.

Useful? React with 👍 / 👎.

🟡 Live harness accidentally sets CONSENSUS_PROCESS_MATCH="undefined" when option not provided

startConsensusServer() always includes an env entry for CONSENSUS_PROCESS_MATCH, even when options.processMatch is undefined. In Node’s spawn, environment values are strings; passing undefined results in the child process receiving the literal string "undefined" (or otherwise a malformed env var), which then gets treated as a regex pattern.

Actual vs expected:

• Actual: the server may run with CONSENSUS_PROCESS_MATCH=undefined, causing new RegExp(matchEnv) to compile and unintentionally match processes containing the word “undefined”, or at minimum diverge from default behavior.
• Expected: when processMatch isn’t provided, the env var should be omitted so normal process detection runs.

Impact:

• Can break or skew live Codex readiness detection and PID/session matching during the new live harness tests.

env: {
  ...process.env,
  CONSENSUS_PORT: String(options.port),
  CONSENSUS_HOST: "127.0.0.1",
  CONSENSUS_CODEX_HOME: options.codexHome,
  CONSENSUS_DEBUG_ACTIVITY: options.debugActivity ? "1" : "0",
  CODEX_TEST_HOOKS: options.testHooks ? "1" : "0",
  CONSENSUS_PROCESS_MATCH: options.processMatch,
},

Recommendation: Only set CONSENSUS_PROCESS_MATCH when options.processMatch is a non-empty string (e.g., build env conditionally, or ...(options.processMatch ? { CONSENSUS_PROCESS_MATCH: options.processMatch } : {})).

Was this helpful? React with 👍 or 👎 to provide feedback.

🟡 Duplicate in-flight timeout guard in Codex tail expiry logic

expireInFlight() contains the exact same guard check twice.

Actual vs expected:

• Actual: redundant condition evaluation every time expireInFlight() runs.
• Expected: a single guard.

Impact:

• Minor: unnecessary code and a small maintainability hazard (future edits could accidentally change only one copy).

if (!state.inFlight) return;
if (!Number.isFinite(inflightTimeoutMs) || inflightTimeoutMs <= 0) return;
if (!Number.isFinite(inflightTimeoutMs) || inflightTimeoutMs <= 0) return;

Recommendation: Remove the duplicated guard line.

Was this helpful? React with 👍 or 👎 to provide feedback.